Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tag cleanup and additions for 1.21 #3807

Merged
merged 9 commits into from
Jun 8, 2024

Conversation

TelepathicGrunt
Copy link
Contributor

@TelepathicGrunt TelepathicGrunt commented May 27, 2024

With 1.21 shaping up to be another large breaking change for many modders, we can use this for our advantage to do some tag cleanup work. This is paired up with this PR: neoforged/NeoForge#1030

The changes includes:

  • Adding a c:music_discs tag since vanilla is removing minecraft:music_discs in 1.21. A log warning was added for this so people know they can migrate over to the c tag for music disc stuff that should not be reliant on the JUKEBOX_PLAYABLE item component.

  • deprecated of c:coal tag that I missed before. I could not find anyone to say why this singular-named tag exists when vanilla has a minecraft:coals tag that would work for modder's use cases. So it seems better to deprecated this tag and add a log warning if found to tell people to migrate to the vanilla tag instead.

  • raw_blocks item tags are deprecated because the storage_blocks tag for the raw blocks already exist and is the same. Log warning added for this migration cleanup. The deprecated of these tags should reduce confusion in this area for modders.

  • Added c:leathers tag on request to match NeoForge side.

  • Removed Eroded Badlands from Sparse Vegetation Overworld since the biome has no trees at all and that normal badllands and desert isn't in the tag in first place.

  • Tool tags were misspelled. So instead of c:tools/bows, it should had been c:tools/bow because tools is the noun to be plural and bow is the adjective. So it would read as Bow Tools instead of the awkward Bows Tools. Attempted to be added in non-breaking way as possible.

  • Deprecated Shulker Boxes block tag because Minecraft has a Shulker Box block tag

  • Glazed Terracotta block/item tag, Concrete block/item tag, Concrete Powder item tag.

Please comment here if there's any change here that you dislike or love! Or if you have suggestions of new tags to add, to sync from other loader, or should also be removed/adjusted.

See the below issue reports for more details and reasoning for each of this PR's changes:

Closes #3742
Closes #3755
Closes #3790
Closes #3731

@TelepathicGrunt
Copy link
Contributor Author

Note there is talk about the wolf variant biome tags on the other PR here:
neoforged/NeoForge#1030 (comment)

Is wolf variant biome tags something Fabric is willing to allow? It's an odd case that requires overriding some vanilla files to work properly in a sensible way. (Original request is #3701)

@TelepathicGrunt
Copy link
Contributor Author

Tool tags were misspelled.

so instead of c:tools/bows, it should had been c:tools/bow because tools is the noun to be plural and bow is the adjective. So it would read as Bow Tools instead of the awkward Bows Tools.

Tried to implement the change as non-breaking as I can. Caused an awkward naming of one field MINING_TOOL_TOOLS because the original field was MINING_TOOLS which is now deprecated

Copy link
Member

@modmuss50 modmuss50 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, many thanks. I will put this into last call, I will aim for it to be slightly shorter than normal (maybe 3 days?) so we can get it in well before the stable release.

@modmuss50 modmuss50 requested a review from a team June 2, 2024 13:17
@modmuss50 modmuss50 added last call If you care, make yourself heard right away! merge me please Pull requests that are ready to merge labels Jun 2, 2024
@modmuss50 modmuss50 merged commit 18dd60f into FabricMC:1.21 Jun 8, 2024
4 checks passed
TelepathicGrunt added a commit to TelepathicGrunt/fabric that referenced this pull request Jun 12, 2024
modmuss50 pushed a commit that referenced this pull request Jun 17, 2024
* Fix new tags not being plural

Fixes #3807

Matches neoforged/NeoForge#1030

* Fixed concrete and terracotta

* Update fabric-convention-tags-v2/src/main/java/net/fabricmc/fabric/api/tag/convention/v2/ConventionalItemTags.java

Co-authored-by: haykam821 <[email protected]>

* Update fabric-convention-tags-v2/src/datagen/java/net/fabricmc/fabric/impl/tag/convention/datagen/generators/EnglishTagLangGenerator.java

Co-authored-by: haykam821 <[email protected]>

* Update fabric-convention-tags-v2/src/datagen/java/net/fabricmc/fabric/impl/tag/convention/datagen/generators/EnglishTagLangGenerator.java

Co-authored-by: haykam821 <[email protected]>

* Update fabric-convention-tags-v2/src/datagen/java/net/fabricmc/fabric/impl/tag/convention/datagen/generators/EnglishTagLangGenerator.java

Co-authored-by: haykam821 <[email protected]>

* Update fabric-convention-tags-v2/src/datagen/java/net/fabricmc/fabric/impl/tag/convention/datagen/generators/EnglishTagLangGenerator.java

Co-authored-by: haykam821 <[email protected]>

* regen data

---------

Co-authored-by: haykam821 <[email protected]>
Su5eD pushed a commit to Sinytra/FAPI-Mojmaps that referenced this pull request Jul 4, 2024
* Fix new tags not being plural

Fixes FabricMC/fabric#3807

Matches neoforged/NeoForge#1030

* Fixed concrete and terracotta

* Update fabric-convention-tags-v2/src/main/java/net/fabricmc/fabric/api/tag/convention/v2/ConventionalItemTags.java

Co-authored-by: haykam821 <[email protected]>

* Update fabric-convention-tags-v2/src/datagen/java/net/fabricmc/fabric/impl/tag/convention/datagen/generators/EnglishTagLangGenerator.java

Co-authored-by: haykam821 <[email protected]>

* Update fabric-convention-tags-v2/src/datagen/java/net/fabricmc/fabric/impl/tag/convention/datagen/generators/EnglishTagLangGenerator.java

Co-authored-by: haykam821 <[email protected]>

* Update fabric-convention-tags-v2/src/datagen/java/net/fabricmc/fabric/impl/tag/convention/datagen/generators/EnglishTagLangGenerator.java

Co-authored-by: haykam821 <[email protected]>

* Update fabric-convention-tags-v2/src/datagen/java/net/fabricmc/fabric/impl/tag/convention/datagen/generators/EnglishTagLangGenerator.java

Co-authored-by: haykam821 <[email protected]>

* regen data

---------

Co-authored-by: haykam821 <[email protected]>
Su5eD pushed a commit to Sinytra/ForgifiedFabricAPI that referenced this pull request Jul 4, 2024
* Fix new tags not being plural

Fixes FabricMC/fabric#3807

Matches neoforged/NeoForge#1030

* Fixed concrete and terracotta

* Update fabric-convention-tags-v2/src/main/java/net/fabricmc/fabric/api/tag/convention/v2/ConventionalItemTags.java

Co-authored-by: haykam821 <[email protected]>

* Update fabric-convention-tags-v2/src/datagen/java/net/fabricmc/fabric/impl/tag/convention/datagen/generators/EnglishTagLangGenerator.java

Co-authored-by: haykam821 <[email protected]>

* Update fabric-convention-tags-v2/src/datagen/java/net/fabricmc/fabric/impl/tag/convention/datagen/generators/EnglishTagLangGenerator.java

Co-authored-by: haykam821 <[email protected]>

* Update fabric-convention-tags-v2/src/datagen/java/net/fabricmc/fabric/impl/tag/convention/datagen/generators/EnglishTagLangGenerator.java

Co-authored-by: haykam821 <[email protected]>

* Update fabric-convention-tags-v2/src/datagen/java/net/fabricmc/fabric/impl/tag/convention/datagen/generators/EnglishTagLangGenerator.java

Co-authored-by: haykam821 <[email protected]>

* regen data

---------

Co-authored-by: haykam821 <[email protected]>
@purejosh
Copy link

purejosh commented Aug 2, 2024

Which tag form is favored, the plural form or the singular form?

After checking the ConventionalItemTags, it appears that the singular form is now preferred.

@TelepathicGrunt
Copy link
Contributor Author

@purejosh which tag are you talking about.nouns are plural. Adjectives are not. So c:tools is plural. c:tools/spear is technically plural as you read it as spear tools. Spear is the adjective for the tool

@purejosh
Copy link

purejosh commented Aug 2, 2024

@purejosh which tag are you talking about.nouns are plural. Adjectives are not. So c:tools is plural. c:tools/spear is technically plural as you read it as spear tools. Spear is the adjective for the tool

Apologies, I posted hastily. I checked the ConventionalItemTags and found my answer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
last call If you care, make yourself heard right away! merge me please Pull requests that are ready to merge
Projects
None yet
3 participants